Branches - Sara + Monick by mfunkemomo · Pull Request #7 · Ada-C12/slack-cli

mfunkemomo · 2019-09-13T21:59:08Z

slack.rb

Congratulations! You're submitting your assignment!

You and your partner should collaborate on the answers to these questions.

Comprehension Questions

Question	Answer
How did you go about exploring the Slack API? Did you learn anything that would be useful for your next project involving an API?	We found Slack API documentation was easy to read and follow. If we had to build something like an app that sends out notifications, this would be useful.
Give a short summary of the request/response cycle. Where does your program fit into that scheme?	The cycle is when the user requesting something from the computer/API. The API then searches for it and returns a value. We did that in our project when we requested a list of channels/users for our workspace from Slack and Slack sent that back to us.
How does your program check for and handle errors when using the Slack API?	We tested if our messages were sent correctly but we did not have time to work API error testing.
Did you need to make any changes to the design work we did in class? If so, what were they?	We surprisingly did not change much in our design. As we worked on our program, we learned that the way we had it worked. In fact, we actually took out a class because it didn't serve a purpose. We realize doing it the way we did, may have been sloppier and if we had time to refactor we would have cleaned it up and utilize another class.
Did you use any of the inheritance idioms we've talked about in class? How?	Yes, our Recipient class was a parent to Channel and User. We were able to use the send_method with Channel and User classes by having the send_message method in Recipient.
How does VCR aid in testing a program that uses an API?	It records all the testings and results that we have from the API when we have internet. When we don't have internet, the VCR will know how to respond to our tests.

… all necessary data

…. Updated tests after refactoring

… message to slackcli and it works

tildeee · 2019-09-21T07:26:45Z

slack.rb

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene (no slack tokens)	yes
Comprehension questions	yes
Functionality
List users/channels	yes
Select user/channel	yes
Show details	yes
Send message	yes
Program runs without crashing	yes
Implementation
API errors are handled appropriately	yes
Inheritance model matches in-class activity	yes
Inheritance idioms (abstract class, template methods, polymorphism) used appropriately	yes
Methods are used to break work down into simpler tasks	didn't see many helper methods
Class and instance methods are used appropriately	yes
Tests written for User functionality	the tests that exist are good, but there are no edge cases tested
Tests written for Channel Functionality	the tests that exist are good, but there are no edge cases tested
Tests written for sending a message	the tests that exist are good, but there are no edge cases tested
Overall

GREAT WORK on this project, Monick and Sara! Your project code is clean, well-written, your test's nominal cases look great, and overall works well. Notably, I think that your slack.rb file has great style, with case statements and good spacing and ideas... there was clearly thought and refactoring there-- well done!

The biggest thing to note is that there aren't any tests that talk about any edge cases; what happens when things go wrong? We didn't go into handling API errors in depth during class, but I'd encourage you two to start thinking about: what happens if my response comes back with a status code that's not 200 OK? Your code currently doesn't handle that and would break in those cases! Food for thought.

That being said, all of my other comments are minor suggestions for how to improve code style.

Overall, this is a great project submission; well done!

tildeee · 2019-09-21T07:27:31Z

lib/channel.rb

+    end 
+
+    def self.list
+      method_url = "https://slack.com/api/channels.list"


Minor suggestion: Could this be a constant variable?

tildeee · 2019-09-21T07:27:43Z

lib/channel.rb

+      channels = response.parsed_response["channels"]
+      i = 0 
+      all_channels = []
+      channels.each do |channel|


Minor suggestion for refactoring: instead of using .each and i and incrementing i, consider using channels.each_with_index do |channel, index|, which has built-in support for an index

tildeee · 2019-09-21T07:27:49Z

lib/recipient.rb

+        channel: recipient,
+        text: message
+      }
+      slack_post = HTTParty.post(method_url, query: query_params)


Something to think about for the future: What happens if this POST request comes back with a response that has an error? Your code currently doesn't handle that!

tildeee · 2019-09-21T07:28:05Z

lib/slack.rb

+    when "select user"
+      ap "Type a username or Slack ID."
+      search_user = gets.chomp
+      selected_recipient = SlackCli::User.list.find do |i|


Minor suggestion: Instead of the variable name i, could we change it to something like recipient or user? This just helps me understand what kinds of elements are in the SlackCli::User.list array

tildeee · 2019-09-21T07:28:42Z

lib/slack.rb

+      selected_recipient = SlackCli::User.list.find do |i|
+        i[:user_name]== search_user || i[:id] == search_user
+      end
+      if selected_recipient == nil 


Minor suggestion: the code selected_recipient.nil? does the same thing as selected_recipient == nil, in case that looks better in your opinion

tildeee · 2019-09-21T07:29:00Z

test/channel_test.rb

+  end 
+end 
+
+describe "channel list method should return correct values" do 


Great tests in here!

tildeee · 2019-09-21T07:29:15Z

test/recipient_test.rb

+    VCR.use_cassette("slack_details") do
+      test = SlackCli::Recipient.send_message(recipient:@selected_recipient.id, message:@message)
+      expect(test.code).must_equal 200
+      expect(test.body).must_include "IT WORKS!"


Honestly, this is a great test overall!

mfunkemomo and others added 14 commits September 10, 2019 14:04

built methods for list_channel, list_users, and user-input interface

6593e4b

Update slack.rb

77a88ac

was able to get env file working

e9c203a

merged changes

9f1b91a

created user, channel, server class and completed user class to print…

cb6bc93

… all necessary data

completed channels class and displaying all necessary info

29649d7

completed user and channel tests. vcr works

c3365d1

refactored channel and user tests

87ac6ef

refactored slack.rb and updated. also began selected user method

1a5e09e

refactored slack.rb, updated with selected user recipient and details…

a0adb7e

…. Updated tests after refactoring

added psuedocode for wave 3

62397bf

created send_message method in recipient class. Has controller send a…

f5571b5

… message to slackcli and it works

added selected channel in controller and started tests for send_message

513a01a

updated slack.rb with notification if recipient was or was not selected

6337b09

tildeee reviewed Sep 21, 2019

View reviewed changes

lib/channel.rb

end

def self.list

method_url = "https://slack.com/api/channels.list"

Copy link

tildeee Sep 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestion: Could this be a constant variable?

tildeee reviewed Sep 21, 2019

View reviewed changes

test/channel_test.rb

end

end

describe "channel list method should return correct values" do

Copy link

tildeee Sep 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great tests in here!

tildeee reviewed Sep 21, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Branches - Sara + Monick#7

Branches - Sara + Monick#7
mfunkemomo wants to merge 14 commits intoAda-C12:masterfrom
mfunkemomo:master

mfunkemomo commented Sep 13, 2019

Uh oh!

tildeee commented Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mfunkemomo commented Sep 13, 2019

slack.rb

Comprehension Questions

Uh oh!

tildeee commented Sep 21, 2019

slack.rb

What We're Looking For

Uh oh!

tildeee Sep 21, 2019

Choose a reason for hiding this comment

Uh oh!

tildeee Sep 21, 2019

Choose a reason for hiding this comment

Uh oh!

tildeee Sep 21, 2019

Choose a reason for hiding this comment

Uh oh!

tildeee Sep 21, 2019

Choose a reason for hiding this comment

Uh oh!

tildeee Sep 21, 2019

Choose a reason for hiding this comment

Uh oh!

tildeee Sep 21, 2019

Choose a reason for hiding this comment

Uh oh!

tildeee Sep 21, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants